.github/workflows/container-build.yml (2)

25-28: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider persist-credentials: false on checkout steps.

zizmor's artipacked finding notes these actions/checkout@v4 steps don't set persist-credentials: false, leaving the ambient GITHUB_TOKEN in .git/config for the remainder of the job — including during cargo build/podman build, which execute third-party build scripts.

🔒 Suggested fix

     - uses: actions/checkout@v4
       with:
         submodules: true
         fetch-depth: 0
+        persist-credentials: false

Also applies to: 54-56

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/container-build.yml around lines 25 - 28, The checkout
steps using actions/checkout@v4 currently leave the ambient GITHUB_TOKEN
persisted in .git/config, which can leak into later cargo build and podman build
steps. Update each checkout block in the workflow to set persist-credentials to
false, including the other checkout occurrence referenced by the review, while
keeping the existing submodules and fetch-depth settings in place.

Source: Linters/SAST tools

---

33-35: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Hoist interpolated expressions into env: to avoid template injection.

zizmor flags direct ${{ inputs.tag-suffix }}, ${{ inputs.make-target }}, and ${{ needs.vars.outputs.* }} interpolation inside run: blocks (lines 33-35, 59, 98-102, 120-124) as template-injection risk. Even though these values currently originate from static strings in ci.yml, the canonical mitigation is to pass them through a step-level env: block and reference them as shell variables, so the pattern remains safe if these inputs are ever driven by less-trusted values.

🔒 Example fix for the `vars` step

     - id: vars
+      env:
+        TAG_SUFFIX: ${{ inputs.tag-suffix }}
       run: |
         cat << EOF >> "$GITHUB_OUTPUT"
-        tag=$(make tag)${{ inputs.tag-suffix }}
-        stackrox-io-image=$(make image-name)${{ inputs.tag-suffix }}
-        rhacs-eng-image=$(FACT_REGISTRY="quay.io/rhacs-eng/fact" make image-name)${{ inputs.tag-suffix }}
+        tag=$(make tag)${TAG_SUFFIX}
+        stackrox-io-image=$(make image-name)${TAG_SUFFIX}
+        rhacs-eng-image=$(FACT_REGISTRY="quay.io/rhacs-eng/fact" make image-name)${TAG_SUFFIX}
         EOF

Similarly for line 59 (env: MAKE_TARGET: ${{ inputs.make-target }} then run: DOCKER=podman make "${MAKE_TARGET}") and the manifest steps (98-102, 120-124), which interpolate needs.vars.outputs.* directly.

Also applies to: 59-59, 98-102, 120-124

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/container-build.yml around lines 33 - 35, The workflow
steps that build image tags and run make targets are interpolating GitHub
expressions directly inside run blocks, which is the template-injection risk
flagged by zizmor. Move the affected values from inputs.tag-suffix,
inputs.make-target, and needs.vars.outputs.* into step-level env entries, then
reference those shell variables inside the run commands. Apply this pattern in
the tag/image-name step, the MAKE_TARGET step, and the manifest-related steps so
the logic stays the same but the expressions are no longer expanded directly in
shell scripts.

Source: Linters/SAST tools

tests/conftest.py (1)

70-100: 🩺 Stability & Availability | 🔵 Trivial

Fixed OTLP port may conflict under parallel test execution.

OtlpServer.serve() defaults to a hardcoded port (4318), mirroring the existing fixed-port pattern for GrpcServer (9999). If these tests are ever run with pytest-xdist or otherwise in parallel across workers, both fixed ports could collide across concurrently-running test processes. Not a regression introduced by this diff, but doubling the fixed-port surface (grpc + otlp) is worth tracking if parallelization is a goal.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/conftest.py` around lines 70 - 100, The server fixture currently
hardcodes OTLP to a fixed port via OtlpServer.serve(), which can collide when
tests run in parallel. Update the server setup in pytest_generate_tests/server
so OtlpServer can be started with a worker-unique or free port (similar to how
GrpcServer is selected by mode), and make the fixture pass that port through
instead of relying on the default 4318. Ensure the change is localized around
_get_output_modes, pytest_generate_tests, and server so each test process gets
an isolated OTLP endpoint.

tests/server.py (1)

41-58: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

ThreadPoolExecutor is never shut down.

Neither GrpcServer.stop() nor OtlpServer.stop() calls self.executor.shutdown(). Each test using the server fixture creates a fresh EventServer subclass instance with its own 2-worker pool that's never explicitly released, leaking idle threads across the test session (they'll only terminate at interpreter exit).

♻️ Proposed fix

     def stop(self):
         """Stop the gRPC server."""
         self.server.stop(1)
         self.running.clear()
+        self.executor.shutdown(wait=False)

and similarly for OtlpServer.stop().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/server.py` around lines 41 - 58, `EventServer` creates a
`ThreadPoolExecutor` in its constructor but the pool is never released, so
update the shutdown path in both `GrpcServer.stop()` and `OtlpServer.stop()` to
call `self.executor.shutdown()` after stopping the server work. Make the cleanup
part of the existing `stop` methods so each `EventServer` subclass instance
properly tears down its worker threads when the test fixture finishes.

fact/src/event/process.rs (1)

45-57: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Inconsistent attribute key naming: exec_path vs exe_path.

Lineage's exe_path field is emitted under the key "exec_path", while the sibling Process conversion (line 259-261 in this same file) emits the analogous field under "exe_path". Since these are exported as OTLP log attributes consumed by external tooling (Grafana/Loki dashboards per this PR's docs), inconsistent naming for the same semantic field across nested maps will confuse queries/dashboards built against the OTel schema.

🔧 Suggested fix

 impl From<Lineage> for opentelemetry::logs::AnyValue {
     fn from(value: Lineage) -> Self {
         AnyValue::Map(Box::new(HashMap::from([
             ("uid".into(), value.uid.into()),
             (
-                "exec_path".into(),
+                "exe_path".into(),
                 value.exe_path.to_string_lossy().to_string().into(),
             ),
         ])))
     }
 }

This isn't exercised by the current Python OTLP test server (which doesn't parse lineage), so it wouldn't be caught by CI as-is.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@fact/src/event/process.rs` around lines 45 - 57, The `Lineage` OTEL
conversion is exporting the `exe_path` field under a different attribute name
than the sibling `Process` conversion, causing inconsistent log schema. Update
the `From<Lineage> for opentelemetry::logs::AnyValue` implementation in
`process.rs` so the map key matches the existing `Process` export naming
(`exe_path`) and keep the attribute naming consistent across both conversions
for downstream queries and dashboards.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes